Skip to content

Significantly improve logbook performance when selecting entities#71657

Merged
balloob merged 17 commits intohome-assistant:devfrom
bdraco:logbook_optimize_entity_call
May 11, 2022
Merged

Significantly improve logbook performance when selecting entities#71657
balloob merged 17 commits intohome-assistant:devfrom
bdraco:logbook_optimize_entity_call

Conversation

@bdraco
Copy link
Copy Markdown
Member

@bdraco bdraco commented May 10, 2022 •
edited
Loading

Breaking change

This change only affects developers using the logbook api.

The entity_matches_only parameter to the logbook api has been been removed as it is no longer necessary. It was originally added for the logbook card to avoid performance problem that this PR has now solved. cc @zsarnett

Proposed change

This PR introduces a new faster query for selecting context ids for specific entities. The makes the logbook card extremely responsive.

Demo of reloading 3 logbook cards
logbook_card

The previous implementation had to process ALL events inside the time window. The new implementation uses a more efficient database query which is able to narrow the rows returned to only ones that have the context id. In production testing this reduced the api call time from ~800ms to ~50ms on a database that generates 100MiB/day. With larger databases the improvement will be at least two orders of magnitude.

All the logbook queries have been separated into queries.py

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@bdraco bdraco requested a review from a team as a code owner May 10, 2022 20:40
@probot-home-assistant
Copy link
Copy Markdown

probot-home-assistant bot commented May 10, 2022

Hey there @home-assistant/core, mind taking a look at this pull request as it has been labeled with an integration (logbook) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@bdraco bdraco mentioned this pull request May 10, 2022
Merged
22 tasks
@zsarnett
Copy link
Copy Markdown
Contributor

zsarnett commented May 10, 2022

Thanks @bdraco I'll take a look at the frontend and PR an update to go with this

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 10, 2022

Screen Shot 2022-05-10 at 16 03 59

The context_id queries are really fast now :)

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 10, 2022 •
edited
Loading

Not counting JSON encode time (which is only ~10% of the overall picture for single enitites), performance breakdown is now

30% ORM overhead
30% DB overhead
40% logbook code (25%+ of which is converting datetime objects to isoformat())

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 11, 2022

Its getting to the point where the http overhead is starting to make converting these apis to websocket attractive

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 11, 2022

Its getting to the point where the http overhead is starting to make converting these apis to websocket attractive

History is actually a better candidate to do first since some of those responses are 100k+ states

#71688

@zsarnett zsarnett mentioned this pull request May 11, 2022
Closed
9 tasks
@bdraco bdraco mentioned this pull request May 11, 2022
Merged
22 tasks
@balloob balloob merged commit 81e8d2a into home-assistant:dev May 11, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 12, 2022
@frenck
Copy link
Copy Markdown
Member

frenck commented May 24, 2022

I just noticed this PR affects developers, but was not published on our development blog (which should have been done according to the latest requirements).

Please publish a blog on the changes on the developer blog @bdraco.

CC @balloob

@bdraco bdraco deleted the logbook_optimize_entity_call branch May 24, 2022 20:46
@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented May 24, 2022

home-assistant/developers.home-assistant#1340

@frenck
Copy link
Copy Markdown
Member

frenck commented May 24, 2022

Awesome, thanks @bdraco ❤️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@balloob balloob balloob approved these changes

Assignees

No one assigned

Labels

breaking-change cla-signed code-quality core has-tests integration: logbook

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bdraco @zsarnett @frenck @balloob @homeassistant